Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Wordlist::get_words_by_prefix for auto-completions and validation #16

Conversation

wigy-opensource-developer

Wordlist.get_word(&self, bits: Bits11) is exported from the crate, but the Bits11 type is not, therefore it is unusable for the UI to give auto-completion or word-by-word validation. I have added a method to query all words that start with a prefix so the UI can decide whether they want to call it once with an empty string and provide auto-completion on their side, or they want to call this method each time the user settles with typing to provide valid continuations.

Thought about starting with a slice::binary_search, but I doubt we should optimize UI code without measuring a problem there.

Copy link
Owner

@maciejhirsz maciejhirsz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this might be a better way of doing this :).

@maciejhirsz
Copy link
Owner

@wigy-opensource-developer on the first point, I think doing list.get_word(u16bits.into()) should work. It wouldn't hurt to turn the signature to take B: Into<Bits11> and have that casting done internally.

@wigy-opensource-developer
Copy link
Author

@wigy-opensource-developer on the first point, I think doing list.get_word(u16bits.into()) should work. It wouldn't hurt to turn the signature to take B: Into<Bits11> and have that casting done internally.

I did not want to extend the PR with this. Just changing the method to

    pub fn get_word(&self, bits: impl Into<Bits11>) -> &'static str {
        self.inner[bits.into().bits() as usize]
    }

causes compilation issues in 3 callers, because the parameter type at the call site will be ambiguous. Because that is unrelated to what this PR is about, I just left it unchanged.

@maciejhirsz maciejhirsz merged commit cb0cc1b into maciejhirsz:master Mar 27, 2020
@maciejhirsz
Copy link
Owner

Published as 0.7.3

@wigy-opensource-developer wigy-opensource-developer deleted the add_get_words_by_prefix branch March 27, 2020 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants